Skip to content

feat(database): CRUD benchmark domain with Postgres - #274

Open
HeyGarrison wants to merge 4 commits into
masterfrom
devin/1785612029-database-crud-benchmark
Open

feat(database): CRUD benchmark domain with Postgres#274
HeyGarrison wants to merge 4 commits into
masterfrom
devin/1785612029-database-crud-benchmark

Conversation

@HeyGarrison

@HeyGarrison HeyGarrison commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

New benchmarks/database/ domain measuring a create → read → update → read → delete cycle, with Postgres as the only provider. Same shape as benchmarks/storage/: types.ts / providers.ts / benchmark.ts / scoring.ts / legacy-results.ts + a declarative crud.bench.ts (config + task, bench run owns the entrypoint, --provider selects one provider). Root scripts bench:database and bench:database:postgres sit next to bench:storage*. Runs against a plain local Postgres container — no cloud credentials.

Abstraction choice: (a), a provider interface local to the domain

There is no database equivalent of @storagesdk/core, so types.ts defines the smallest interface the workload needs, mirroring storage's createStorage() with a createClient() factory on the provider config:

interface DatabaseClient {
  setup(): Promise<void>;                 // create table/collection, once per participant, untimed
  create(doc: DatabaseDocument): Promise<void>;
  read(id: string): Promise<DatabaseDocument | null>;
  update(id, patch: Pick<DatabaseDocument, 'name'|'payload'|'version'>): Promise<void>;
  delete(id: string): Promise<number>;    // rows removed — doubles as delete verification
  close(): Promise<void>;
}

Why not something else: publishing a package was out of scope, and a query-level abstraction (raw SQL, or an ORM like Drizzle) would not carry over to MongoDB/Firestore, which is the whole point of the registry. Everything provider-specific — the pg.Pool, the table DDL, the parameterised statements — lives in postgres.ts behind this interface, so adding MongoDB later is one entry in providers.ts plus one client module, and swapping in a real SDK later means reimplementing createClient only. delete returning a count rather than void lets the cycle assert the row is gone without an extra untimed round trip.

Workload

Each phase is its own ctx.step and separately timed into data (createMs, readMs, updateMs, readAfterUpdateMs, deleteMs, totalMs, payloadBytes), following how storage reports uploadMs/downloadMs, so the platform can chart per-phase latency. Reads are verified, not just timed: the post-create read must match the written document and the post-update read must observe the new version/payload, otherwise the iteration fails with DATABASE_ERROR. Payload size is a --payload-size flag parsed from argv the same way storage parses --file-size (default 1 KiB).

Env vars

  • DATABASE_POSTGRES_URL (required)
  • DATABASE_BENCH_TABLE (optional, default benchmark_crud)
docker run --rm --name database-benchmark-postgres -p 5433:5432 \
  -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=benchmark postgres:16
DATABASE_POSTGRES_URL=postgresql://postgres:postgres@127.0.0.1:5433/benchmark pnpm bench:database:postgres

tsconfig.json gains benchmarks/database/**/*.ts — the include list is per-domain, so without it pnpm typecheck would silently skip every new file.

Local run

pnpm typecheck passes. Run end to end against a local Postgres (5433) with a local benchmarks-platform stack (per .agents/skills/local-platform-e2e) as the reporting target; the platform's ClickHouse import of this run also succeeded (imported: 1, failed: 0, records: 10, steps: 50).

> tsx packages/benchsdk-runner/dist/bin.js run benchmarks/database/crud.bench.ts --provider postgres --payload-size 1024

Database CRUD (local) (self-contained)
Knobs: iterations=10, concurrency=1, staggerDelayMs=0, groupBy=participant

Run created: 2c83bad6-b0d8-480b-88ec-48d0cf1b61fd
======================================================================
  Participant: postgres
======================================================================
  [postgres] Task 1/10: success {"createMs":6.687896,"readMs":1.478172,"updateMs":0.958039,"readAfterUpdateMs":2.871194,"deleteMs":0.73118,"totalMs":13.013246,"payloadBytes":1024}
  [postgres] Task 2/10: success {"createMs":0.644397,"readMs":0.599691,"updateMs":4.261675,"readAfterUpdateMs":0.784018,"deleteMs":0.657754,"totalMs":7.040833,"payloadBytes":1024}
  [postgres] Task 3/10: success {"createMs":0.514186,"readMs":0.574232,"updateMs":0.568213,"readAfterUpdateMs":0.481009,"deleteMs":1.532547,"totalMs":3.758292,"payloadBytes":1024}
  [postgres] Task 4/10: success {"createMs":0.625934,"readMs":0.431872,"updateMs":3.817789,"readAfterUpdateMs":2.656433,"deleteMs":1.263329,"totalMs":8.912508,"payloadBytes":1024}
  [postgres] Task 5/10: success {"createMs":0.946772,"readMs":0.480355,"updateMs":1.260775,"readAfterUpdateMs":0.534825,"deleteMs":0.513826,"totalMs":3.839468,"payloadBytes":1024}
  [postgres] Task 6/10: success {"createMs":0.526541,"readMs":0.448021,"updateMs":0.40638,"readAfterUpdateMs":0.330008,"deleteMs":0.367934,"totalMs":2.164837,"payloadBytes":1024}
  [postgres] Task 7/10: success {"createMs":0.477204,"readMs":0.389678,"updateMs":0.462877,"readAfterUpdateMs":0.40343,"deleteMs":0.52297,"totalMs":2.371259,"payloadBytes":1024}
  [postgres] Task 8/10: success {"createMs":0.4964,"readMs":0.901417,"updateMs":0.549923,"readAfterUpdateMs":0.420781,"deleteMs":0.416062,"totalMs":2.876798,"payloadBytes":1024}
  [postgres] Task 9/10: success {"createMs":0.487472,"readMs":0.373107,"updateMs":0.414609,"readAfterUpdateMs":0.348502,"deleteMs":0.395219,"totalMs":2.080819,"payloadBytes":1024}
  [postgres] Task 10/10: success {"createMs":0.455437,"readMs":0.455036,"updateMs":0.548529,"readAfterUpdateMs":0.455374,"deleteMs":0.530378,"totalMs":2.519572,"payloadBytes":1024}
  Done: 10/10 succeeded.

Results written to results/database/2026-08-01.json
Copied latest: results/database/latest.json

Task 1 carries the cold-connection cost; the generated results/database/*.json are not committed.

Link to Devin session: https://app.devin.ai/sessions/2fd0b29d80ee433eb61bc9d7e015ed80
Requested by: @HeyGarrison


Open in Devin Review

HeyGarrison and others added 3 commits August 1, 2026 19:24
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@HeyGarrison HeyGarrison self-assigned this Aug 1, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@open-cla

open-cla Bot commented Aug 1, 2026

Copy link
Copy Markdown

Contributor License Agreement

All contributors are covered by a CLA.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment on lines +100 to +107
} catch (error) {
try {
await client.delete(id);
} catch {
// Best-effort cleanup after a failed cycle.
}
throw error;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 A failed database cycle can hang the whole benchmark forever during cleanup

The leftover record is removed (client.delete(id) at benchmarks/database/benchmark.ts:102) with no time limit after a failed cycle, so an unresponsive database leaves the benchmark stuck with no way to finish.

Impact: A single hung cleanup call blocks the entire run indefinitely instead of failing the iteration and moving on.

Why the timeout protection is missing on this path

Every timed phase is wrapped by withTimeout through the step shim in benchmarks/database/crud.bench.ts:77, but the best-effort cleanup inside runCrudCycle's catch block calls the client directly, bypassing that wrapper. The equivalent storage benchmark explicitly wraps its failure-path cleanup: benchmarks/storage/storage.bench.ts:107-108 uses withTimeout(storage!.delete(key), 10_000, 'Delete timed out').

Because the Postgres pool is configured with max: 1 (benchmarks/database/postgres.ts:24), a cleanup delete that never resolves also blocks the single connection for all remaining iterations.

Prompt for agents
In benchmarks/database/benchmark.ts, the catch block of runCrudCycle performs a best-effort cleanup delete by calling client.delete(id) directly, with no timeout. All timed phases go through the step shim in benchmarks/database/crud.bench.ts which wraps calls in withTimeout, so this cleanup is the only unbounded database call in the workload. The Postgres client uses a pool with max: 1, so a hung cleanup also starves every later iteration. Consider bounding the cleanup call with the shared withTimeout helper (benchmarks/src/util/timeout.ts), mirroring how benchmarks/storage/storage.bench.ts bounds its failure-path delete at 10s; this may require passing a timeout (or a pre-wrapped cleanup function) into runCrudCycle.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in b7ec88a. The failure-path cleanup now goes through a ctx.cleanup() wrapper that crud.bench.ts implements with withTimeout(..., 10_000, 'Delete timed out'), matching benchmarks/storage/storage.bench.ts. Kept the helper out of benchmark.ts so the cycle stays runner-agnostic, and cleanup errors (including that timeout) are still swallowed so the original workload error is what gets rethrown.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant